fix(rules): improve precision and recall of java opengrep rules - #112

Addresses a customer SAST evaluation that reported roughly 90% false
positives from the Java rules and compared them unfavourably to CodeQL.

Reproduced on six mature open source Java projects (guava, netty,
spring-framework, commons-lang, commons-io, spring-petclinic, ~17,400
Java files): the rule set emitted 1,631 findings, and a hand adjudicated
random sample of 40 contained zero true positives. Three rules produced
74% of that volume.

Precision fixes:
- java-empty-catch-block: 645 findings, all noise. Restrict to swallowed
  broad exceptions, exclude the conventional "ignored"/"expected" variable
  names, and exclude blocks carrying an explanatory comment. Comments are
  not AST nodes, so a documented catch block still looks empty to the
  matcher and had to be excluded textually.
- java-reflection-injection: 296 findings. Was matching every
  method.invoke(), every newInstance() factory call, and Class.forName()
  on string constants. Converted to taint mode with servlet and Spring MVC
  sources and dynamic-class-loading and script-eval sinks.
- java-system-out-usage: 263 findings. The message claims sensitive data
  in logs but the rule matched any println. Now requires the printed
  expression to reference something credential bearing.
- java-hardcoded-credentials: matched on variable name alone, flagging
  KEY_ATTRIBUTE = "key" and SEC_WEBSOCKET_KEY1. Ported the value
  inspection approach already applied to the dotnet rules in SocketDev#63: bare
  "key" only counts in compound credential words, and values shaped like
  header names, property paths, or a restatement of the keyword itself
  are excluded. Now zero findings across all six mature libraries while
  still catching WebGoat's default credentials.
- java-unsafe-deserialization: required the receiver to actually be an
  ObjectInputStream, and excluded calls inside a class's own readObject
  and readExternal implementations, which are the Serializable contract.
- java-insecure-random: converted to taint mode. A weak PRNG is only a
  vulnerability when its output becomes a security value, not when it
  seeds a JMH benchmark or shuffles a list.
- java-hardcoded-ip: required a full dotted quad and excluded loopback.
- java-insecure-cookie: bound the setSecure(true) exclusion to the same
  variable, so one hardened cookie no longer exonerates every other
  cookie in the method.

Recall fixes. Two systematic bugs suppressed entire categories:
- Patterns using simple type names never matched fully qualified call
  sites, so java.security.MessageDigest.getInstance("MD5"),
  new java.util.Random() and new javax.servlet.http.Cookie() were all
  invisible. Added qualified variants throughout.
- Crypto rules matched exact algorithm literals, so
  Cipher.getInstance("DES/CBC/PKCS5Padding") did not match "DES". Replaced
  with metavariable-regex over the transformation string, and covered the
  provider overloads of getInstance.

Also added java-xss and java-xpath-injection, both taint mode, and
converted java-ldap-injection and java-path-traversal to taint with
Zip Slip and Spring multipart sources.

Validated with opengrep 1.25.0.

OWASP Benchmark v1.2 (2,740 annotated cases, ground truth):
  precision  64.5% -> 76.5%
  recall     12.4% -> 63.4%
  score       5.1  -> 42.6
  securecookie, weakrand, crypto and hash reach 100% precision.

Mature open source Java projects:
  1,631 -> 130 findings (-92%)
  unique findings on mature libraries 1,536 -> 85 (-94.5%)

WebGoat: 87 -> 45. The removed findings are lint noise and three
reflection matches on factory calls; the planted vulnerabilities,
including the Zip Slip, default credentials and weak PRNG, still fire.

Methodology, per-category results, the known limits of OWASP Benchmark
for pattern-based engines, and the remaining untouched noise sources are
documented in docs/java-sast-benchmark.md, with a reusable scorer in
scripts/score_owasp_benchmark.py.

Six of Bugbot's seven findings reproduced and are fixed; the seventh (the
readObject throws clause) did not reproduce and is dismissed. Five further
defects found in review are fixed alongside them.

Precision:
- java-weak-cipher: RSA/ECB/PKCS1Padding was reported as a broken ECB cipher.
  In a JCA RSA transformation "ECB" is a placeholder, not a block mode, and is
  the standard spelling. AES/ECB/... is still reported.
- java-insecure-cookie: the exclusion region ran from one cookie's declaration
  to its setSecure call, so a second cookie constructed inside that region was
  dropped whenever a neighbour was hardened. The positive pattern is now a
  declaration, which forces $COOKIE to unify between the match and the
  exclusion. Added an assignment form so hardening through a field is
  recognised, and a branch for a cookie constructed inline and never assigned.
- java-insecure-random: the sink name regex was unanchored, so the short words
  matched as substrings of ordinary identifiers: iv in pivot and divisor, pin
  in spinner, key in monkey, auth in author. The short words now require a word
  boundary and seed is dropped entirely. Two details worth remembering:
  metavariable-regex anchors at the start of the name, so each alternation
  branch needs its own leading .*, and a leading global (?i) also lowercased
  the deliberately case-sensitive camelCase branch, which is what let those
  substrings through.
- java-unsafe-deserialization: a SnakeYAML load using
  new Yaml(new SafeConstructor()) was reported even though that is the
  remediation the rule's own fix text recommends.
- java-ldap-injection: an untyped $CTX.search(...) sink turned a Lucene
  IndexSearcher.search() into a CRITICAL LDAP finding. Sinks are now type
  constrained. Dropping the untyped sink initially halved Benchmark recall,
  which turned out to be the same qualified-name bug fixed elsewhere in this
  branch: the corpus declares javax.naming.directory.InitialDirContext and only
  the simple name was covered. Recall is restored at 77.8%.
- java-reflection-injection: $ENUM.valueOf(...) also matched String.valueOf,
  so taint laundered through a plain string conversion escaped detection.
  Integer.valueOf and Long.valueOf remain sanitizers.

Recall:
- java-hardcoded-ip: the 10 branch allowed only two more octets, so "10.0.0.1"
  was missed while the version string "10.2.3" was reported. Requires a full
  dotted quad.
- java-insecure-random: Random.nextBytes is void and fills the caller's array,
  so tainting the call expression never reached SecretKeySpec or
  IvParameterSpec. Added a by-side-effect source focused on the argument.
  weakrand now scores 100% precision at 100% recall.
- java-path-traversal: the startsWith containment sanitizer lacked
  by-side-effect, so it only cleaned the startsWith expression itself and the
  checked variable stayed tainted at every later sink. The containment check
  suppressed nothing.
- java-hardcoded-credentials: the hyphen exclusion treated any hyphenated
  lowercase value as a header name, which dropped sk-live-... and xoxb-... style
  keys. Digits are the discriminator: header names essentially never contain
  one. Added explicit exclusions for x- prefixed names and HTTP auth scheme
  words, and widened the restatement check to underscores so "access_token" and
  "j_password" are excluded.

Tooling and docs:
- Added Java rule regression fixtures under tests/fixtures/opengrep/java with
  // ruleid: and // ok: annotations, and tests/test_java_opengrep_rules.py to
  score them. Every defect above is covered. The tests skip when opengrep is
  absent. Note that opengrep's default ignore list skips any directory named
  tests/, so the harness passes explicit file paths.
- scripts/score_owasp_benchmark.py: added a usage guard, set the exec bit, and
  removed the java-trust-boundary-violation mapping for a rule that does not
  exist, so trustbound reads as unscored rather than 0% recall.
- Pinned the engine version and the BenchmarkJava commit in the doc, added the
  two newly confirmed known limits (a SecureRandom held in a Random-typed
  variable, and the statement-scoped comment exclusion in
  java-empty-catch-block), and re-ran every number.
- Added a CHANGELOG entry.

Re-measured on opengrep 1.25.0, BenchmarkJava at 51f0a7c:
  precision  64.5% -> 76.8%  (was 76.5% before this commit)
  recall     12.4% -> 68.7%  (was 63.4%)
  score       5.1  -> 47.2   (was 42.6)
  securecookie and weakrand both 100% precision at 100% recall.

Mature open source Java projects: 1,631 -> 129 findings, and 1,536 -> 84
unique findings on the mature libraries alone. WebGoat 87 -> 45.

Full suite: 344 passed. opengrep --validate clean at 32 rules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

An independent re-review of 87b814f found that three of the review fixes had
over-corrected and one was incomplete. Verified each against a probe before
changing anything; all four reproduced.

Regressions introduced by the previous commit, now fixed:

- java-insecure-random: anchoring the short credential words fixed the
  substring matches but broke leading position. otpCode, pinNumber,
  keyMaterial and every field assignment (this.key, this.otp, this.pin) stopped
  being reported, while shardKey still was. The word-boundary check now allows
  a camelCase suffix and a field-access prefix. pivot, divisor, spinner, monkey
  and author remain clean.
- java-path-traversal: typing the startsWith sanitizer receiver. The untyped
  form let any String.startsWith sanitize, so a bypassable blacklist such as
  name.startsWith("..") suppressed the finding, in either branch and with any
  argument. Path.startsWith is component-wise containment; String.startsWith is
  a prefix test. Only the former is a sanitizer now.
- java-hardcoded-credentials: widening the restatement exclusion to underscores
  also swallowed real weak defaults, dropping "password123", "secret_2024" and
  a planted WebGoat credential. Reused the digit discriminator already applied
  to the hyphen rule: a value that restates the field name never carries a
  digit.
- java-unsafe-deserialization: the SafeConstructor exclusion only matched the
  no-arg constructor, which SnakeYAML 2.0 removed. It now accepts arguments, so
  new Yaml(new SafeConstructor(new LoaderOptions())) is excluded.

Defects predating this branch, found by the same review:

- java-sql-injection: the untyped $TEMPLATE.update(...) sink matched
  MessageDigest.update(input), producing 93 CRITICAL findings on the
  Benchmark's hash test cases. Typing the receiver was not workable, because
  the template is routinely reached through a static field, so the crypto
  receivers are subtracted instead. Added queryForRowSet and batchUpdate as
  sinks while there. sqli recall 44.1% -> 52.2% and precision 64.9% -> 66.0%.
- java-insecure-random and java-ldap-injection were missing qualified-name
  variants (java.util.Random, javax.naming.ldap.InitialLdapContext). This is
  the defect class the branch claims to fix throughout.

Tests and docs:

- Fixtures cover every regression above: leading short words, field
  assignment, a qualified java.util.Random receiver, a String.startsWith
  blacklist, "password123", and SafeConstructor(LoaderOptions). The Lucene
  fixture was passing a value through Integer.parseInt, which is a listed
  sanitizer, so it guarded nothing; it now reaches the sink unparsed.
- Added a test asserting every finding lands on an annotated line, so an
  annotation cannot be satisfied by an unrelated finding covering the same line.
- Doc corrections: the Before column now comes from the current scorer
  (recall 13.2%, score 5.6; the old Before counted the 126 trustbound cases
  against recall on one side only), and it discloses the cross-category
  findings the OWASP method discards, the 26% of baseline volume that sits in
  now-excluded test and example paths, the duplicated guava android mirror, and
  the +70% scan time. Added known limits for the ambiguity of "key" as a name
  and for the deserialization helpers that remain.

Re-measured on opengrep 1.25.0, BenchmarkJava at 51f0a7c:
  precision  64.5% -> 76.7%
  recall     13.2% -> 70.3%
  score       5.6  -> 48.2
  true positives 176 -> 937
  securecookie and weakrand 100% precision at 100% recall.

Mature open source Java projects: 1,631 -> 129 findings, 1,536 -> 84 unique on
the mature libraries. WebGoat 87 -> 45.

Full suite: 345 passed. opengrep --validate clean at 32 rules.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…s portable

The regression tests added for the Java rules skipped in CI, because the
python-tests workflow never installed opengrep, so the fixtures guarded
nothing. The workflow now installs the release pinned by OPENGREP_VERSION in
the Dockerfile and sets SOCKET_BASICS_REQUIRE_OPENGREP=1, so a missing engine
fails the job instead of silently skipping the module. Rule and fixture paths
are added to the workflow's path filters, since a java.yml-only change did
not trigger it before.

The harness scans a temporary copy of the fixtures and asserts that every
fixture was scanned and that opengrep reported no errors. opengrep's default
ignore list skips any path under tests/, and on 1.19.0 that applies even to
explicitly listed files, so scanning in place returned zero files and every
positive annotation failed as if the rules had regressed.

Drop -a from the scan command. It is --autofix, inert today only because
every fix: key in java.yml sits under metadata.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ocketDev#111 and SocketDev#112

SocketDev#111 merged to main with its own [Unreleased] block, so the PR stopped being
mergeable and GitHub could not compute a merge ref, which is why no
pull_request workflow ran on the previous head. The two blocks are combined
section by section (Added, Changed, Removed, Fixed); nothing else in the file
differs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

java-weak-crypto-sha1 listed DigestUtils.sha1Hex and the fully qualified
sha1, but not org.apache.commons.codec.digest.DigestUtils.sha1Hex, so that
spelling was missed while the MD5 rule covered all four. Adds a weak-hash
fixture covering every spelling of both rules. OWASP Benchmark is unchanged.

Of the four findings in this Bugbot round only this one reproduces. The
other three (toLowerCase/replace in java-xss, StringBuilder.toString in
java-reflection-injection, Long.toHexString in java-insecure-random) are
reported on the current rules: opengrep propagates taint through any method
call on a tainted receiver and any call with a tainted argument by default,
so the explicit String propagators are belt-and-braces. The benchmark doc now
says so, to save the next review round the same detour.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ocketDev#110 and SocketDev#112

SocketDev#110 merged to main with its TruffleHog entries under [Unreleased], so the PR
was unmergeable again. Main's block is kept as the base and only the bullets
tagged SocketDev#112 are appended per section; nothing else in the file differs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>